Hi, any news on this merge request? I've not checked whether it's fixed in 19.0.3 or 20.x though but I'm a bit stuck for the annual closure. It there any workaround for the mean time?

I can test the closing with patches from this MR since I reproduce 100% the issue.

I did some investigation around the issue, this triggers the BookkeepingRecordAlreadyExists error with the details on auxiliary accounts enabled. After some additional logs, I get :

2024-10-07 13:19:23 NOTICE      --- Access to GET /accountancy/closure/index.php - action=confirm_step_2, massaction=
2024-10-07 13:19:23 WARNING       BookKeeping::create BookkeepingRecordAlreadyExists: $row->nb =1 for doc_type=closure numero_compte=1068 label_operation=Exercice 2024
2024-10-07 13:19:23 INFO        --- End access to /accountancy/closure/index.php

But when I check for any bookkeeping entries in llx_accounting_bookkeeping with either doc_type=closure or label_operation like Exercice%, I don't get any entries, so I suppose it comes from entries being added multiple tests

So I guessed what is happening is that it might create multiple entries with the same label and numero_compte , which is indeed the case:

2024-10-07 13:27:59 WARNING       BookKeeping::create Creating new bookkeeping entry: $row->id=0 for doc_type=closure numero_compte=1013 label_operation=Exercice 2024
2024-10-07 13:27:59 WARNING       BookKeeping::create Creating new bookkeeping entry: $row->id=0 for doc_type=closure numero_compte=1061 label_operation=Exercice 2024
2024-10-07 13:27:59 WARNING       BookKeeping::create Creating new bookkeeping entry: $row->id=0 for doc_type=closure numero_compte=1068 label_operation=Exercice 2024
2024-10-07 13:27:59 WARNING       BookKeeping::create BookkeepingRecordAlreadyExists: $row->id=1 for doc_type=closure numero_compte=1068 label_operation=Exercice 2024

Is grouping those "A-nouveau" or finding a way to create them uniquely in an identifiable manner the root cause of the issue?

Ok I did some investigation without the auxiliary account details yesterday evening and I understand better where the MR is going.

Without the option enabled, I had those errors:

2024-10-07 13:58:03 WARNING       BookKeeping::create Creating new bookkeeping entry: $row->nb=0 entity=1 fk_doc=2 doc_type=closure numero_compte=4421 label_compte=PRELEVEMENT A LA SOURCE label_operation=Exercice 2024
2024-10-07 13:58:03 WARNING       BookKeeping::create Creating new bookkeeping entry: $row->nb=1 entity=1 fk_doc=2 doc_type=closure numero_compte=4421 label_compte=PRELEVEMENT A LASOURCE label_operation=Exercice 2024
2024-10-07 13:58:03 WARNING       BookKeeping::create BookkeepingRecordAlreadyExists: $row->nb=1 entity=1 fk_doc=2 doc_type=closure numero_compte=4421 label_operation=Exercice 2024

A previous bookkeeping operation got an incorrect PRELEVEMENT A LASOURCE label, instead of PRELEVEMENT A LA SOURCE. I guess it was coming from how the account was named at the creation date. Then the account name was fixed but not the operation itself.

I also had another instance of the issue where some operations were labelled as "BANQUE XXX", and others with the same account labelled "BANQUE YYY". The original was that the "BANQUE XXX" was bought by "BANQUE YYY" and changed name, so the bank name was refleted back by modifying the bank record on the bank management page. But then.

In both those case, because the label was different, the grouping created two entries, so changing the grouping to not account for label_compte seems like a good solution here:

-				$sql .= ' GROUP BY t.numero_compte, t.label_compte, aa.pcg_type';
+				$sql .= ' GROUP BY t.numero_compte, aa.pcg_type';

Likewise, doing a fallback to the current value of the account seems like the correct solution if the original code doesn't supply a specific operation name (that might be changed depending on the configuration of the account), instead of using the one from the original bookkeeping posting (that cannot be updated)

+				if (empty($this->label_compte)) {
+					$accountingaccount = new AccountingAccount($this->db);
+					$accountingaccount->fetch('', $this->numero_compte);
+
+					dol_syslog(get_class($this).":: fetch label_compte if empty for number =".$accountingaccount->label, LOG_DEBUG);
+
+					$this->label_compte = $accountingaccount->label;
+				}

I've not checked if the MR would fix the issue on my case, but I'd say it would given the analysis. I will test that during the week. In the mean time, I think I can also write a test mapping the problem I encountered for non-regression testing if it can be integrated to the MR or after the MR.

Hi @alexandre-janniaux and @aspangaro and @eldy
I did further testing and investigation and I found several issues:

• different label_compte with same account number
• removing label_compte is raising an issue and the code in the line around 2770 $bookkeeping->label_compte = $obj->label_compte;
• different subledger_label with same subledger_account
• empty versus null values for subledger_label and subledger_account? What is the rule by default?
• What to do when the value of opening_balance is 0 as it creates a bookkeeping entry for now. (I will get a return from my accounting expert this morning) We could add this on the SQL resquest or add a if before creating the booking line
  HAVING
  (SUM(t.credit) - SUM(t.debit)) != 0 -- Exclude rows with opening_balance = 0
• How duplicate bookkeeping have to be handle (see my other PR that I am investigating too FIX - Update Accounting closure with missing too many A-Nouveau #30039)

I already wrote a code to fix all of them but I need advises/guidelines from our experts here on:

• are you approving generating the label_compte and subledger_label based on the lastest label of the numero_compte and subledger_account number
• changing the SQL request by the following one to fix the above issues as below:
  SELECT
  t.numero_compte,
  NULLIF(t.subledger_account, '') as subledger_account,
  -- NULLIF(t.subledger_label, '') as subledger_label,
  aa.pcg_type,
  (SUM(t.credit) - SUM(t.debit)) as opening_balance
  FROM
  llx_accounting_bookkeeping as t
  LEFT JOIN
  llx_accounting_account as aa
  ON aa.account_number = t.numero_compte
  WHERE
  t.entity = 1
  AND aa.entity = 1
  AND aa.fk_pcg_version IN (
  SELECT pcg_version
  FROM llx_accounting_system
  WHERE rowid = 50
  )
  AND aa.pcg_type IN ('CAPIT','IMMO','STOCK','THIRDPARTY','FINAN','EXPENSE','INCOME')
  AND DATE(t.doc_date) >= '2023-01-01 00:00:00'
  AND DATE(t.doc_date) <= '2023-12-31 00:00:00'
  GROUP BY
  t.numero_compte,
  aa.pcg_type,
  NULLIF(t.subledger_account, '')
  -- NULLIF(t.subledger_label, '')
  HAVING
  (SUM(t.credit) - SUM(t.debit)) != 0 -- Exclude rows with opening_balance = 0
  ORDER BY
  t.numero_compte ASC

Here are the issues and results in the SQL request moving from 2132 rows to 325:

Hi, sorry for the answer delay. I'm not sure I'll be able to answer everything but, first, disclamer:

• I am not an accountant, I mainly only do plain text accounting on my free time, so what follows is the "logical" understand that I have of it, and might not necessarily apply.

are you approving generating the label_compte and subledger_label based on the lastest label of the numero_compte and subledger_account number

I guess what we really want is the current value in the configuration (the only source where we don't have a possible confusion since they are unique).

In short, the bookkeeping entries were generated with labels coming from the AccountingAccount. They cannot be updated, but the new entries cannot have both values anyway so using the alledgedly corrected one is probably better.

Ideally, we would not have different label in the data model but on the technical side, it implies being able to modify previous bookkeeping records.

What to do when the value of opening_balance is 0 as it creates a bookkeeping entry for now. (I will get a return from my accounting expert this morning) We could add this on the SQL resquest or add a if before creating the booking line

No clue on my side, but having the empty bookeeping can serve as proof of processing (to check later that every account have been transfered) and reference (origin point, if we want to compare exercices). Maybe this still has a usage?

I will submit another PR to accelerate the fix whatever solution will be chosen.

Thanks a lot for the efforts, that's really appreciated. 🙏